Skip to content
This repository has been archived by the owner on Oct 16, 2023. It is now read-only.

Improve generating the minified code #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Arkkimaagi
Copy link
Contributor

I noticed that the minified code did not get updated when I did the improvement to support multiple trigger buttons.

As I could not find documentation on how it's generated, I created this pull request to use uglify-js for minifying via npm script. I also added a bit of documentation so that others would not have to wonder as much how to contribute.

  • Add uglify-js to minify javascript
  • Add npm run uglify script to generate the minified (and unminified version for debugging) version
  • Add contribution documentation
  • Bump up version number to 3.4.1

@Arkkimaagi
Copy link
Contributor Author

Maybe after this a new release could be created?

@scottaohara
Copy link
Owner

hey again.

Actually, I've been meaning to just delete the minified JS so that people could just use whatever minification / build process they want to.

@Arkkimaagi
Copy link
Contributor Author

I think removing the minified version might be a bad idea.

If you check the google inert code, it too has minified code for ready use.

People might want to just get the minified version directly without always building a minifying workflow to their system. This way people do not have to build their own tooling for simple use cases.

This pull request is about having a simple tooling in the project for minifying the code automatically helps with contributions.

@scottaohara
Copy link
Owner

i understand the intent of the PR. It just conflicts with what I was planning on doing here.

I'll think on it.

However, i definitely don't see value in copying over the unminified index.js file into the assets folder.

@Arkkimaagi
Copy link
Contributor Author

Yep, maybe copying the unminified can be removed. Usually when you concatenate several scripts to one, it's a good idea to create so I did it out of habit. But here the need for that is not as clear.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants